Skip to content

Cleanup in Experiment Server - #14185

Merged
andreas-el merged 6 commits into
equinor:mainfrom
andreas-el:experiment_server_touch_up
Aug 20, 2026
Merged

Cleanup in Experiment Server#14185
andreas-el merged 6 commits into
equinor:mainfrom
andreas-el:experiment_server_touch_up

Conversation

@andreas-el

@andreas-el andreas-el commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Issue
Resolves #13927

Fixed typos
Removed unused constants
Update enum mapping
Fix comments

  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure unit tests pass locally after every commit (git rebase -i main --exec 'just rapid-tests')

When applicable

  • When screenshots are changed: Review screenshot-PR in ert-testdata,
    merge screenshot-PR in ert-testdata before merging this PR.
  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Add backport label to latest release (format: 'backport release-branch-name')

@andreas-el andreas-el added the release-notes:skip If there should be no mention of this in release notes label Aug 14, 2026
@andreas-el
andreas-el force-pushed the experiment_server_touch_up branch from d9b3bdf to e448461 Compare August 14, 2026 13:16
@codecov-commenter

codecov-commenter commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.95%. Comparing base (3eb1fd7) to head (9eb7dcd).
⚠️ Report is 17 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...rc/ert/dark_storage/endpoints/experiment_server.py 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14185      +/-   ##
==========================================
+ Coverage   91.93%   91.95%   +0.02%     
==========================================
  Files         482      484       +2     
  Lines       33513    33639     +126     
==========================================
+ Hits        30809    30933     +124     
- Misses       2704     2706       +2     
Flag Coverage Δ
cli-tests 36.41% <0.00%> (-0.06%) ⬇️
fuzz 44.37% <0.00%> (-0.09%) ⬇️
gui-tests 58.69% <0.00%> (-0.14%) ⬇️
performance-and-unit-tests 80.96% <0.00%> (+0.08%) ⬆️
test 45.96% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/everest/strings.py 100.00% <ø> (ø)
...rc/ert/dark_storage/endpoints/experiment_server.py 62.60% <0.00%> (+0.27%) ⬆️

... and 13 files with indirect coverage changes

@codspeed-hq

codspeed-hq Bot commented Aug 17, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing andreas-el:experiment_server_touch_up (9eb7dcd) with main (293883c)

Open in CodSpeed

@berland

berland commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

The last commit does not look like a cleanup. Can you elaborate what kind of issue it solves? (since it adds complexity to the code)

@andreas-el

Copy link
Copy Markdown
Contributor Author

The last commit does not look like a cleanup. Can you elaborate what kind of issue it solves? (since it adds complexity to the code)

I can put that in a separate PR. The issue listed code smells, including global variables. The last commit tries to battle exactly that, and this is surely up for debate if the change actually improves code quality, readability etc.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Cleans up Experiment Server state management and removes obsolete Everest constants.

Changes:

  • Replaces module-global experiment state with an injectable cached registry.
  • Passes experiment state directly to runners and tightens exit-code handling.
  • Updates tests and removes unused constants.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/ert/dark_storage/endpoints/experiment_server.py Refactors registry injection and runner state handling.
src/everest/strings.py Removes unused constants.
tests/everest/test_everserver.py Updates tests for the registry accessor.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/ert/dark_storage/endpoints/experiment_server.py Outdated
Comment thread src/ert/dark_storage/endpoints/experiment_server.py Outdated
@eqbech

eqbech commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

I can put that in a separate PR. The issue listed code smells, including global variables. The last commit tries to battle exactly that, and this is surely up for debate if the change actually improves code quality, readability etc.

I agree we can put this in a separate PR, and the. we can look into whether we can use lifespan for this global state issue.

@andreas-el
andreas-el force-pushed the experiment_server_touch_up branch 2 times, most recently from 035a791 to 168d9fd Compare August 19, 2026 13:25
@andreas-el
andreas-el force-pushed the experiment_server_touch_up branch from 168d9fd to 9eb7dcd Compare August 19, 2026 13:35
@andreas-el
andreas-el enabled auto-merge (rebase) August 19, 2026 13:36
@andreas-el
andreas-el merged commit 6078659 into equinor:main Aug 20, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes:skip If there should be no mention of this in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clean up endpoints found in experiment_server

5 participants